-
Notifications
You must be signed in to change notification settings - Fork 618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the zlib "inflate" implementation from the image library #223
Conversation
and instead use the external zlib implementation from https://github.com/sivadeilra/zlib. This is better for refactoring dependencies, and also quite substantially improves performance for decoding PNG images.
I have safe Rust code (written more than 6 months ago IIRC) that borderline outperforms zlib. You definitely do not need |
Well great then I've wasted a week. Where's this fast code? -----Original Message----- I have safe Rust code (written more than 6 months ago IIRC) that borderline outperforms zlib. You definitely do not need goto for state machines. |
I suppose he means a newer version of this code from #99? Never-the-less we would still need a fast deflater or do you have something fast on the shelf as well @eddyb? Btw. that's a classic and reminds me to always file an issue when implementing something big. Funnily I also did some work in this direction. :( @sivadeilra I skimmed over your lib. What is the performance difference between a save version of your code and the unsafe one? You didn't put cfg-guards around all of them. I also noted quite some panics, would it be possible to replace them with a If it's safe and faster what we have currently I do not see any reason to not include the code until someone comes up with something even faster and safe. |
@nwin that's the code I am talking about, it should be failrly easy to update. But I don't have a deflater, I wouldn't know where to even start with one. |
FWIW this version compiles with the latest rust. I can't guarantee it works because there were some nasty type-errors because the integers literals did not have any annotations and I'm not sure that I always picked the right type. The error messages were really funny sometimes… |
Hey, I'd like to resume this conversation. Got distracted by a few things. First, about the "panic!" calls. Right now, just because my zlib port is Second, about performance. I don't mean that "goto" is necessary for One important design question is, should the Rust port of zlib provide a I took a look at the inflate.rs that was posted in the gist. It's All I care about is having a great zlib implementation -- I don't care who So, what would you guys like to do? I can work in improving my zlib to Thoughts? On Sat, Jan 3, 2015 at 4:26 AM, nwin notifications@github.com wrote:
|
I don't have an opinion about that. Nowadays I would guess in most use cases you don't stream images although it is of course neat if it is possible. |
Closing because of outdated. |
This removes the zlib "inflate" implementation from the image library, and and instead uses the external zlib implementation from https://github.com/sivadeilra/zlib. This is better for refactoring dependencies, and also quite substantially improves performance for decoding PNG images.